Skip to content

Bunny/Total configuration conflict check #1234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: total-cdn-staging
Choose a base branch
from
Open

Bunny/Total configuration conflict check #1234

wants to merge 2 commits into from

Conversation

@jacobd91
Copy link
Contributor

@jacobd91 jacobd91 commented Oct 28, 2025 •
edited
Loading

This PR does 2 things

  1. It fixes the existing conflict check for Bunny so you can't configure both CDN and FSD with bunny at the same time. The check had a bug where if you have both disabled and try to enable both, it would block the second enable and show a warning. If you enable 1 and save, on page load if you then try to enable the 2nd it allowed it. This PR fixes that bug
  2. It adds the same check for Total CDN as to prevent users from configuring CDN and FSD with Total CDN

One thing to note is that the conflict check prevents mixing Bunny and Total for CDN/FSD

To test simply visit the General Settings page and attempt to configure. On conflict it should revert the last changed setting and show a warning

  • Bunny CDN | Bunny FSD
  • Total CDN | Total FSD
  • Bunny CDN | Total FSD
  • Total CDN | Bunny FSD

Lastly the conflict check should prevent the Total FSD confirmation popup from executing if the configuration is in conflict

@jacobd91
Fixed bug with bunny CDN where you could enable both CDN and FSD unde…
00e5133 
…r certain conditions. Added missing check for total CDN to prevent enabling both CDN and FSD.
@jacobd91 jacobd91 requested review from Copilot and cssjoe October 28, 2025 18:58
@jacobd91 jacobd91 self-assigned this Oct 28, 2025
Copilot AI reviewed Oct 28, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors CDN conflict detection to prevent enabling Bunny CDN and Total CDN simultaneously for both CDN and Full Site Delivery (FSD). The implementation uses capture-phase event listeners to intercept changes before other handlers execute, and maintains a state tracker to revert conflicting selections.

  • Extended conflict detection from Bunny CDN only to include Total CDN and mixed configurations
  • Replaced the old cdn_bunnycdn_check() with a comprehensive cdn_conflict_check() system using capture-phase event listeners
  • Added server-side validation in Generic_AdminActions_Default.php to block conflicting configurations on save

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
pub/js/options.js Replaced single-engine conflict check with comprehensive CDN/FSD conflict detection system using capture-phase listeners and state management
Generic_Plugin_Admin.php Added warning messages for Total CDN and mixed CDN conflicts; updated Bunny CDN warning text
Generic_AdminActions_Default.php Added server-side validation to prevent saving conflicting CDN configurations
Cdn_TotalCdn_FsdEnablePopup.js Removed file (functionality replaced by refactored conflict detection system)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 2.28%. Comparing base (4740f7a) to head (ec06320).
⚠️ Report is 2 commits behind head on total-cdn-staging.

Files with missing lines Patch % Lines
Generic_AdminActions_Default.php 0.00% 28 Missing ⚠️
Generic_Plugin_Admin.php 0.00% 13 Missing ⚠️
Additional details and impacted files 
@@                   Coverage Diff                   @@
##             total-cdn-staging    #1234      +/-   ##
=======================================================
- Coverage                 2.28%    2.28%   -0.01%     
- Complexity               20218    20224       +6     
=======================================================
  Files                      669      669              
  Lines                   102088   102127      +39     
=======================================================
  Hits                      2335     2335              
- Misses                   99753    99792      +39

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cssjoe
cssjoe requested changes Nov 7, 2025
View reviewed changes
Copy link
Member

@cssjoe cssjoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

Generic_Plugin_Admin.php
),
'pull_zone' => __( 'Pull Zone could not be automatically created.', 'w3-total-cache' ),
'cdn_fsd_conflict_bunnycdn' => esc_html__( 'Bunny CDN cannot be enabled for both CDN and Full Site Delivery. Please disable one and save again.', 'w3-total-cache' ),
'cdn_fsd_conflict_totalcdn' => esc_html__( 'Total CDN cannot be enabled for both CDN and Full Site Delivery. Please disable one and save again.', 'w3-total-cache' ),
Copy link
Member

@cssjoe cssjoe Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

Generic_Plugin_Admin.php
'pull_zone' => __( 'Pull Zone could not be automatically created.', 'w3-total-cache' ),
'cdn_fsd_conflict_bunnycdn' => esc_html__( 'Bunny CDN cannot be enabled for both CDN and Full Site Delivery. Please disable one and save again.', 'w3-total-cache' ),
'cdn_fsd_conflict_totalcdn' => esc_html__( 'Total CDN cannot be enabled for both CDN and Full Site Delivery. Please disable one and save again.', 'w3-total-cache' ),
'cdn_fsd_conflict_mixed' => esc_html__( 'Bunny CDN and Total CDN cannot be enabled across CDN and Full Site Delivery at the same time. Please disable one and save again.', 'w3-total-cache' ),
Copy link
Member

@cssjoe cssjoe Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

Generic_Plugin_Admin.php
),
'updated_pullzone_url' => __( 'Pull Zone URL could not be automatically updated. Please contact support for assistance.', 'w3-total-cache' ),
'cdn_totalcdn_fsd_origin_update_failed' => __( 'Unable to update the Total CDN origin for Full Site Delivery. Please contact support for assistance.', 'w3-total-cache' ),
'cdn_totalcdn_fsd_origin_update_failed' => __( 'Unable to update the Total CDN origin for Full Site Delivery. Please contact support for assistance.', 'w3-total-cache' ),
Copy link
Member

@cssjoe cssjoe Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

Generic_Plugin_Admin.php
'w3-total-cache'
),
'totalCdnWarning' => esc_html__(
'Total CDN cannot be enabled for both CDN and Full Site Delivery.',
Copy link
Member

@cssjoe cssjoe Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

Generic_Plugin_Admin.php
'w3-total-cache'
),
'mixedCdnWarning' => esc_html__(
'Bunny CDN and Total CDN cannot be enabled across CDN and Full Site Delivery at the same time.',
Copy link
Member

@cssjoe cssjoe Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use W3TC_CDN_NAME instead of "Total CDN".

@cssjoe cssjoe added this to the 2.9.0 (tcdn) milestone Nov 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@cssjoe cssjoe cssjoe requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@jacobd91 jacobd91

Labels

None yet

Projects

None yet

Milestone

2.9.0 (tcdn)

Development

Successfully merging this pull request may close these issues.

4 participants

@jacobd91 @codecov-commenter @cssjoe